-
Notifications
You must be signed in to change notification settings - Fork 541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display success message in Verification Suite Result #375
base: master
Are you sure you want to change the base?
Display success message in Verification Suite Result #375
Conversation
…assertOn value makes it easier to see which verification succeeded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution, we appreciate the changes. Just one clarification needed.
"Success", "Value: 1.0 does meet the constraint requirement!"), | ||
("group-2-E", "Error", "Error", "SizeConstraint(Size(None))", "Failure", | ||
"Value: 4 does not meet the constraint requirement! Should be greater than 5!"), | ||
("group-2-E", "Error", "Error", "CompletenessConstraint(Completeness(att2,None))", | ||
"Success", ""), | ||
"Success", "Value: 1.0 does meet the constraint requirement! Should equal 1!"), | ||
("group-2-W", "Warning", "Warning", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify why the message contains the hint ("Should ...!") in one case, but not in the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, the reason is that the first check does not actually have a hint. In that case only the standard message is shown.
The checks are in the same test suite from line 201:
private[this] def getChecks(): Seq[Check] = {
val checkToSucceed = Check(CheckLevel.Error, "group-1")
.isComplete("att1")
val checkToErrorOut = Check(CheckLevel.Error, "group-2-E")
.hasSize(_ > 5, Some("Should be greater than 5!"))
.hasCompleteness("att2", _ == 1.0, Some("Should equal 1!"))
val checkToWarn = Check(CheckLevel.Warning, "group-2-W")
.hasDistinctness(Seq("item"), _ < 0.8, Some("Should be smaller than 0.8!"))
checkToSucceed :: checkToErrorOut :: checkToWarn :: Nil
}
The change allows the message and hint to be passed through even if the evaluation was a success, before that the message and the hint was not displayed. Passing it through makes it a bit easier if the verification result is saved, because they can be used to know what the threshold at a given time was.
Thanks for clarification. One last request: can we change the message for the success case to |
…ror message as discussed in PR.
Sorry for the delay, was moving house. Makes sense, I updated the message. |
Hi @twollnik , given that change is done, just wanted to check if you need anything else. Apologies again for providing the change for that a bit late. |
#371
Display also a success message for verification, seeing the hint and assertOn value makes it easier to see which verification succeeded.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.